Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: replace deprecated datatype.uuid() with string.uuid() #2214

Merged
merged 2 commits into from
Jun 16, 2023
Merged

docs: replace deprecated datatype.uuid() with string.uuid() #2214

merged 2 commits into from
Jun 16, 2023

Conversation

saranatour1
Copy link
Contributor

No description provided.

@saranatour1 saranatour1 requested a review from a team June 15, 2023 17:54
@saranatour1 saranatour1 requested a review from a team as a code owner June 15, 2023 17:54
@saranatour1 saranatour1 changed the title Refactor: deprecated function from faker.datatype.uuid() to faker.string.uuid() refactor: deprecated function from faker.datatype.uuid() to faker.string.uuid() Jun 15, 2023
@saranatour1 saranatour1 reopened this Jun 15, 2023
@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent labels Jun 15, 2023
@ST-DDT ST-DDT changed the title refactor: deprecated function from faker.datatype.uuid() to faker.string.uuid() docs: replace deprecated datatype.uuid() with string.uuid() in README Jun 15, 2023
ST-DDT
ST-DDT previously approved these changes Jun 15, 2023
View reviewed changes
@ST-DDT ST-DDT requested review from a team June 15, 2023 18:31
@ST-DDT
Copy link
Member

ST-DDT commented Jun 15, 2023

https://github.com/faker-js/faker/blob/next/docs/guide/usage.md also has a few references to the old method.
Would you like to change them in this PR as well?

@codecov
Copy link

codecov bot commented Jun 15, 2023 •
edited
Loading

Codecov Report

Merging #2214 (51d3af1) into next (631f732) will increase coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files 
@@           Coverage Diff           @@
##             next    #2214   +/-   ##
=======================================
  Coverage   99.59%   99.60%           
=======================================
  Files        2607     2607           
  Lines      245002   245002           
  Branches     1152     1155    +3     
=======================================
+ Hits       244017   244031   +14     
+ Misses        958      944   -14     
  Partials       27       27

see 1 file with indirect coverage changes

@saranatour1 saranatour1 dismissed stale reviews from xDivisionByZerox and ST-DDT via 51d3af1 June 15, 2023 19:05
@saranatour1
Copy link
Contributor Author

saranatour1 commented Jun 15, 2023 •
edited
Loading

I did that but while I was reading, I noticed that this method has been marked as deprecated as well :
Link , and It was used in the same docs,
please let me know if I need to close this PR or just submit another one.
Thanks!
@ST-DDT

@saranatour1 saranatour1 reopened this Jun 15, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Jun 15, 2023

Link , and It was used in the same docs,
please let me know if I need to close this PR or just submit another one.

helpers.unique is only soft deprecated and there isn't a replacement yet, so there is no need to replace it yet.

@ST-DDT ST-DDT changed the title docs: replace deprecated datatype.uuid() with string.uuid() in README docs: replace deprecated datatype.uuid() with string.uuid() Jun 15, 2023
@ST-DDT ST-DDT requested review from a team June 15, 2023 19:11
@saranatour1
Copy link
Contributor Author

Do I submit another PR?

@xDivisionByZerox
Copy link
Member

Sorry, I don't understand. Why do you want to open another PR exactly? 🤔

@saranatour1
Copy link
Contributor Author

They Mentioned that there were other changes in another document, I changed them and this is why I am asking.

@xDivisionByZerox
Copy link
Member

Ah gotcha. @ST-DDT changed the PR title so it better reflects all changes done in the PR.
So to conclude: The changes are similar enough that they can be reviewed/merged in the same PR.

Thank you for your contribution ❤️

@ST-DDT ST-DDT merged commit b9008bf into faker-js:next Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xDivisionByZerox xDivisionByZerox xDivisionByZerox approved these changes

@ST-DDT ST-DDT ST-DDT approved these changes

@matthewmayer matthewmayer matthewmayer approved these changes

Assignees

@saranatour1 saranatour1

Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
v8.x
Development

Successfully merging this pull request may close these issues.
4 participants
@saranatour1 @ST-DDT @xDivisionByZerox @matthewmayer